Changes between main and next for review #2421

krlmlr · 2025-10-26T14:46:37Z

These are the essential difference between the main and next branches. This is not intended to be merged, just as a review tool.

Do any of these differences look suspicious? Let's discuss here and resolve.

Generated with:

git diff next -- ':(exclude)man' ':(exclude)src/vendor' ':(exclude)patch' ':(exclude)src/Makevars.*' ':(exclude)src/rinterface.*' ':(exclude)src/cpp11.cpp' ':(exclude)NEWS.md' ':(exclude)R/aaa-auto.R' ':(exclude)cran-comments.md' ':(exclude)tools/stimulus/' ':(exclude)src/sources-igraph.mk' | patch -p1 -R

@szhorvat @schochastics: We do benefit from taking a closer look here, but we will also catch errors later. I'll review too.

krlmlr

Looks solid to me, by and large. Appreciate a second pair of eyes.

krlmlr · 2025-10-26T14:47:36Z

R/centrality.R

+  if (lifecycle::is_present(directed)) {
+    if (directed) {
+      lifecycle::deprecate_soft(
+        "2.2.0",


Here and everywhere:

Suggested change

"2.2.0",

"3.0.0",

krlmlr · 2025-10-26T14:48:51Z

R/centralization.R

+#' centr_degree(g, normalized = FALSE)$centralization /
+#'   centr_degree_tmax(g, loops = "twice")


I remember I wasn't sure about this. For centr_degree(), the default is "twice" IIRC. Is then the same loops value needed in the denominator?

Yes, this is correct.

krlmlr · 2025-10-26T14:49:50Z

R/console.R

-    )
-  }
-  0L
+  # Catch all errors


Could also go into main .

krlmlr · 2025-10-26T14:50:58Z

R/rewire.R

  if (!is(with, "igraph_rewiring_method")) {
    cli::cli_abort(
-      "{.arg with} must be an igraph rewiring method, 
+      "{.arg with} must be an igraph rewiring method,


Should be in main.

krlmlr · 2025-10-26T14:51:59Z

R/simple.R

+#' @param directed Boolean.Whether to consider the directions of edges.
+#' The default, `TRUE`  means that edge directions will be considered.


Suggested change

#' @param directed Boolean.Whether to consider the directions of edges.

#' The default, `TRUE` means that edge directions will be considered.

#' @param directed Boolean. Whether to consider the directions of edges.

#' The default, `TRUE` means that edge directions will be considered.

krlmlr · 2025-10-26T14:54:06Z

R/structural-properties.R

 #' @rdname degree
 #' @export
 #' @cdocs igraph_maxdegree
-max_degree <- maxdegree_impl


Could ask Copilot, could manually backport to main .

Two more functions that were forgotten: mean_distance() and sample_spanning_tree() .

Copilot

Pull Request Overview

This PR contains essential differences between the main and next branches for review. The changes primarily focus on updating function signatures, parameter names, and implementations to align with underlying igraph C library updates.

Key changes:

Standardization of parameter names (e.g., directed → mode, loops from boolean to character)
Updates to match C library API changes (attribute handling, function signatures)
Addition of new function parameters and deprecation of old ones

Reviewed Changes

Copilot reviewed 28 out of 29 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/testthat/test-structural-properties.R	Updates test parameters to use new loop handling syntax
tests/testthat/test-simple.R	Adds test for `is_simple()` with directed parameter
tests/testthat/test-interface.R	Adds snapshot tests for `incident()` function
tests/testthat/test-centrality.R	Adds warning suppression for `eigen_centrality()`
tests/testthat/test-aaa-auto.R	Removes tests for deprecated functions, updates parameter names
tests/testthat/_snaps/*.md	Updates snapshots to match new function behaviors and error messages
src/rinterface_extra.c	Major updates to match C library API changes for attributes and functions
R/*.R	Updates R function signatures, adds deprecations, parameter renaming

Copilot · 2025-10-26T14:59:28Z

src/rinterface_extra.c

+    PROTECT(app=R_igraph_strvector_to_SEXP(tmprec->value.as_strvector));
+    break;
+  default:
    break;


The addition of a default case that does nothing could lead to app being used uninitialized if an unexpected attribute type is encountered. Consider either initializing app to R_NilValue before the switch, or handling the default case with an error.

Suggested change

break;

igraph_error("Unknown attribute type, internal error", __FILE__, __LINE__,

IGRAPH_EINVAL);

return R_NilValue;

Copilot · 2025-10-26T14:59:28Z

src/rinterface_extra.c

 }

-igraph_error_t R_igraph_interrupt_handler(void *data) {
+igraph_bool_t R_igraph_interrupt_handler(void) {


The return type changed from igraph_error_t to igraph_bool_t, and the function signature removed the void *data parameter. Ensure all call sites have been updated accordingly to match this new signature.

Copilot · 2025-10-26T14:59:28Z

R/structural-properties.R

+#' `"twice"` (the default) counts each loop edge twice in undirected graphs
+#' and once in directed graphs.


Line 883 is missing proper punctuation. It should either end with a period or be properly connected to line 882. The current formatting suggests it's a continuation but lacks proper grammatical structure.

Suggested change

#' `"twice"` (the default) counts each loop edge twice in undirected graphs

#' and once in directed graphs.

#' `"twice"` (the default) counts each loop edge twice in undirected graphs and once in directed graphs.

Copilot · 2025-10-26T14:59:29Z

R/centralization.R

+#'   `"twice"` counts each loop edge twice in undirected graphs.
+#' and once in directed graphs.


Line 394 appears orphaned from the previous line's structure. It should either be joined to line 393 or reformatted as a continuation of the bullet point structure.

Suggested change

#' `"twice"` counts each loop edge twice in undirected graphs.

#' and once in directed graphs.

#' `"twice"` counts each loop edge twice in undirected graphs and once in directed graphs.

Changes between main and next for review

cf4b411

krlmlr requested review from schochastics and szhorvat October 26, 2025 14:46

krlmlr commented Oct 26, 2025

View reviewed changes

krlmlr requested a review from Copilot October 26, 2025 14:58

Copilot AI reviewed Oct 26, 2025

View reviewed changes

		#' centr_degree(g, normalized = FALSE)$centralization /
		#' centr_degree_tmax(g, loops = "twice")

		#' @param directed Boolean.Whether to consider the directions of edges.
		#' The default, `TRUE` means that edge directions will be considered.

-    break;
+    igraph_error("Unknown attribute type, internal error", __FILE__, __LINE__,
+                 IGRAPH_EINVAL);
+    return R_NilValue;

		#' `"twice"` (the default) counts each loop edge twice in undirected graphs
		#' and once in directed graphs.

		#' `"twice"` counts each loop edge twice in undirected graphs.
		#' and once in directed graphs.

Uh oh!

Changes between main and next for review #2421

Are you sure you want to change the base?

Changes between main and next for review #2421

Uh oh!

Conversation

krlmlr commented Oct 26, 2025

Uh oh!

krlmlr left a comment

Choose a reason for hiding this comment

Uh oh!

krlmlr Oct 26, 2025

Choose a reason for hiding this comment

Uh oh!

krlmlr Oct 26, 2025

Choose a reason for hiding this comment

Uh oh!

szhorvat Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

krlmlr Oct 26, 2025

Choose a reason for hiding this comment

Uh oh!

krlmlr Oct 26, 2025

Choose a reason for hiding this comment

Uh oh!

krlmlr Oct 26, 2025

Choose a reason for hiding this comment

Uh oh!

krlmlr Oct 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 26, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants